@dblock @dlvenable the PoC for getting rid of the dependency on OpenSearch core, basically more or less the amount of code we would need to copy / paste from core (I think I could reduce it a bit but that would not be a game changer there). It should help us to assess the go / no-go decision.

I love it. It's a lot smaller than I thought it would be and it's mostly boilerplate (+ tests?).

Sure, a few sketches needed to make it compete and testable (just wanted to hear your opinion first), I'll wrap it up shortly.

@dblock @VachaShah got stuck on this one #47 :(

I love this as well. Removing the dependency on core clearly outweighs any downside to code duplication, in my opinion.

@dblock @andrross @VachaShah I think the POC is ready

This is real progress!

What's the path to delete https://github.com/opensearch-project/opensearch-java/blob/f1fbe833d7cd925018b7ed3f9cd1c0684e25b993/java-client/build.gradle.kts#LL149C5-L149?

Thanks @dblock ! This is what we have discussed here [1], basically easy steps:

• for 3.0.0, make RestClient optional (as the dependency)
• recommend to use Apache HttpClient 5 based transport instead

[1] opensearch-project/OpenSearch#5424 (comment)

This is real progress!
What's the path to delete https://github.com/opensearch-project/opensearch-java/blob/f1fbe833d7cd925018b7ed3f9cd1c0684e25b993/java-client/build.gradle.kts#LL149C5-L149?

Thanks @dblock ! This is what we have discussed here [1], basically easy steps:

* for 3.0.0, make RestClient optional (as the dependency)

* recommend to use Apache HttpClient 5 based transport instead

[1] opensearch-project/OpenSearch#5424 (comment)

I may be confused, but that's the plan for removing the legacy rest client from OpenSearch right? Why can't we remove the dependency on OpenSearch from opensearch-java right now?

This is real progress!
What's the path to delete https://github.com/opensearch-project/opensearch-java/blob/f1fbe833d7cd925018b7ed3f9cd1c0684e25b993/java-client/build.gradle.kts#LL149C5-L149?

Thanks @dblock ! This is what we have discussed here [1], basically easy steps:

* for 3.0.0, make RestClient optional (as the dependency)

* recommend to use Apache HttpClient 5 based transport instead

[1] opensearch-project/OpenSearch#5424 (comment)

I may be confused, but that's the plan for removing the legacy rest client from OpenSearch right? Why can't we remove the dependency on OpenSearch from opensearch-java right now?

That would be hard breaking change: since we never deprecated RestClientTransport, just removing it without any notice is probably not a good idea. We at least could start with deprecating RestClientTransport, and removing in the next version, I think that would be less impactful.

Thanks @reta @andrross and @dlvenable! Let's talk about what we want to merge. I think we should

1. Make this PR as "added OpenSearchTransport", document how to use it, merge
2. In a separate PR, "deprecate RestClientTransport (and possibly other classes)", and mark the deprecated classes as such, document the recommended way to use the client ([FEATURE] Deprecate RestClientTransport #326)
3. Backport 1) and 2) to 2.x, continue a 2.x line of releases.
4. In 3.0/main delete the deprecated code.

Is it possible to do this in a way where the public-facing part of the API doesn't have to change when the underlying Apache HttpClient changes? i.e. call the public API ApacheHttpClientTransport That way users don't have to change their code when the dependency is updated.

This is not really possible, the Apache HttpClient 4 and Apache HttpClient 5 are very different: we could hide some (like fe HttpHost) but everything from credentials to TLS configuration does require package change. However, if we do ApacheHttpClient4Transport in 2.x, we could keep it in 3.x as well - zero change for users.

Is it possible to do this in a way where the public-facing part of the API doesn't have to change when the underlying Apache HttpClient changes? i.e. call the public API ApacheHttpClientTransport That way users don't have to change their code when the dependency is updated.

This is not really possible, the Apache HttpClient 4 and Apache HttpClient 5 are very different: we could hide some (like fe HttpHost) but everything from credentials to TLS configuration does require package change. However, if we do ApacheHttpClient4Transport in 2.x, we could keep it in 3.x as well - zero change for users.

Makes sense. I'm slowly getting up to speed here :) So HttpClient 4 and 5 are actually distinct packages that can exists side-by-side (org.apache.httpcomponents.httpclient vs org.apache.httpcomponents.client5.httpclient5). Is that right? Assuming that's true, why is it a breaking change to add the new HttpClient 5 dependency in 2.x?

 Makes sense. I'm slowly getting up to speed here :) So HttpClient 4 and 5 are actually distinct packages that can exists side-by-side (org.apache.httpcomponents.httpclient vs org.apache.httpcomponents.client5.httpclient5). Is that right?

That's right.

Assuming that's true, why is it a breaking change to add the new HttpClient 5 dependency in 2.x?

This is true. There are no concerns regarding breaking change, my concern is "new dependencies are going to be introduced", it may be very surprising to see that within minor version update (may be I am overly cautious).

Assuming that's true, why is it a breaking change to add the new HttpClient 5 dependency in 2.x?

This is true. There are no concerns regarding breaking change, my concern is "new dependencies are going to be introduced", it may be very surprising to see that within minor version update (may be I am overly cautious).

The tl;dr is that this PR will add a new dependency (Apache HttpClient 5) and create a new ApacheHttpClient5Transport class that will be the new recommended approach for creating a client, as opposed to the existing RestClientTransport that depends on OpenSearch core. The specific question is whether adding the HttpClient5 dependency should be considered a breaking change, or can it be added in the next minor release on 2.x? @wbeckler @dlvenable @dblock What do you think?

Assuming that's true, why is it a breaking change to add the new HttpClient 5 dependency in 2.x?

This is true. There are no concerns regarding breaking change, my concern is "new dependencies are going to be introduced", it may be very surprising to see that within minor version update (may be I am overly cautious).

The tl;dr is that this PR will add a new dependency (Apache HttpClient 5) and create a new ApacheHttpClient5Transport class that will be the new recommended approach for creating a client, as opposed to the existing RestClientTransport that depends on OpenSearch core. The specific question is whether adding the HttpClient5 dependency should be considered a breaking change, or can it be added in the next minor release on 2.x? @wbeckler @dlvenable @dblock What do you think?

Please note that right now Apache HttpClient 5 comes with OpenSearch core 3.0.0, nothing is added to opensearch-java (as you can see, the build dependencies have not changed in this pull request).

@reta , I did not actually see a dependency change in this PR (no modification to the build file). It seems that we are pulling in Apache HTTP Client 5 via a transitive dependency.

We may want to explicitly add this dependency now that the project itself depends on it.

Thanks @reta, this looks good to me!

I believe the remaining open question is whether to backport this to 2.x (with a slight change to explicitly declare the HttpClient5 dependencies) or instead introduce an ApacheHttpClient4Transport. In either case this change will be needed on main.

Is this dependency already in the 2.x line? If so, we are not adding a new dependency. If not, we can make this optional at runtime to address the concern of adding new dependencies. End users would need to add it to their build files to use the transport.

The specific question is whether adding the HttpClient5 dependency should be considered a breaking change, or can it be added in the next minor release on 2.x?

In my opinion, this is not breaking since it is compatible with HttpClient4. If we really want, it is possible to make this an optional dependency on 2.x using feature variants. End users would have to add this dependency or get a runtime error.

@dlvenable We are getting HttpClient5 on the main branch via a transitive dependency. For the 2.x branch it would have to be added a new dependency.

I merged it. @reta backport or not?

I merged it. @reta backport or not?

Let me try to do this on Monday (the automatic one won't work, we need explicit dependencies), thanks @dblock !